Skip to content

8380282: [lworld] Problem-list several SA/jhsdb tests when running with --enable-preview#2236

Closed
Arraying wants to merge 4 commits intoopenjdk:lworldfrom
Arraying:JDK-8380282
Closed

8380282: [lworld] Problem-list several SA/jhsdb tests when running with --enable-preview#2236
Arraying wants to merge 4 commits intoopenjdk:lworldfrom
Arraying:JDK-8380282

Conversation

@Arraying
Copy link
Copy Markdown
Member

@Arraying Arraying commented Mar 17, 2026

Hi,

The following tests fail with --enable-preview and have been bulk problemlisted:

resourcehogs/serviceability/sa/TestHeapDumpForLargeArray.java
serviceability/sa/CDSJMapClstats.java
serviceability/sa/ClhsdbDumpheap.java
serviceability/sa/ClhsdbJhisto.java
serviceability/sa/ClhsdbJstackWithConcurrentLock.java
serviceability/sa/TestJmapCore.java
serviceability/sa/TestJmapCoreMetaspace.java
sun/tools/jhsdb/HeapDumpTest.java
sun/tools/jhsdb/HeapDumpTestWithActiveProcess.java
sun/tools/jhsdb/JShellHeapDumpTest.java

Testing on Linux (x64, AArch64), macOS (x64, AArch64), Windows x64:

  • tier 1 sanity test
  • :hotspot_serviceability (most of the SA tests), with and without preview
  • :hotspot_resourcehogs (some SA tests), with and without preview
  • :svc_tools (some jhsdb tests), with and without preview

Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed (1 review required, with at least 1 Committer)

Issue

  • JDK-8380282: [lworld] Problem-list several SA/jhsdb tests when running with --enable-preview (Bug - P4)

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/valhalla.git pull/2236/head:pull/2236
$ git checkout pull/2236

Update a local copy of the PR:
$ git checkout pull/2236
$ git pull https://git.openjdk.org/valhalla.git pull/2236/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2236

View PR using the GUI difftool:
$ git pr show -t 2236

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/valhalla/pull/2236.diff

Using Webrev

Link to Webrev Comment

@bridgekeeper
Copy link
Copy Markdown

bridgekeeper bot commented Mar 17, 2026

👋 Welcome back phubner! A progress list of the required criteria for merging this PR into lworld will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk
Copy link
Copy Markdown

openjdk bot commented Mar 17, 2026

@Arraying This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8380282: [lworld] Problem-list several SA/jhsdb tests when running with --enable-preview

Reviewed-by: coleenp, cjplummer

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 125 new commits pushed to the lworld branch:

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the lworld branch, type /integrate in a new comment.

@openjdk openjdk bot changed the title 8380282 8380282: [lworld] Skip SA tests when running with --enable-preview Mar 17, 2026
@Arraying Arraying marked this pull request as ready for review March 17, 2026 16:25
@openjdk openjdk bot added the rfr Pull request is ready for review label Mar 17, 2026
@mlbridge
Copy link
Copy Markdown

mlbridge bot commented Mar 17, 2026

Webrevs

@plummercj
Copy link
Copy Markdown
Contributor

plummercj commented Mar 17, 2026

What is the motivation for disabling all the SA tests instead of problem listing just the failing tests? How many SA tests are failing with --enable-preview?

@coleenp
Copy link
Copy Markdown
Contributor

coleenp commented Mar 18, 2026

I would 100% much rather we programmatically disable them than problem list a list of tests. The ProblemList list is noise that I thought I cleaned out once but it keeps changing. This is a lot better. Also if this fixes this test to not run, it should be removed from the problem list.

ProblemList-enable-preview.txt:serviceability/sa/TestHeapDumpForInvokeDynamic.java                                              8377387 generic-all

@alexmenkov
Copy link
Copy Markdown

alexmenkov commented Mar 18, 2026

Hi @alexmenkov, thanks for making a comment in an OpenJDK project!

All comments and discussions in the OpenJDK Community must be made available under the OpenJDK Terms of Use. If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please Use "Add GitHub user alexmenkov" for the summary.

If you are not an OpenJDK Author, Committer or Reviewer, simply check the box below to accept the OpenJDK Terms of Use for your comments.

Your comment will be automatically restored once you have accepted the OpenJDK Terms of Use.

@coleenp
Copy link
Copy Markdown
Contributor

coleenp commented Mar 18, 2026

I feel like this makes extra work in categorizing and triaging issues for something that is going to continue to break in different ways all the time. Maybe we could have one issue to reenable the tests and fix them, if we plan on fixing these in the future.

@plummercj
Copy link
Copy Markdown
Contributor

It is best to detect new failures as they happen, not months down the road when someone decides to disable this change and retest, and then has to figure out what changes caused the new failures.

It looks like only 8 tests out of about 90 tests are failing. Most of the failures I skimmed over already have bugs filed for them. We should get them all problem listed so we have clean results and can then readily detect new failures.

@Arraying Arraying changed the title 8380282: [lworld] Skip SA tests when running with --enable-preview 8380282 Mar 24, 2026
@openjdk openjdk bot changed the title 8380282 8380282: [lworld] Problem-list several SA/jhsdb tests when running with --enable-preview Mar 24, 2026
@Arraying
Copy link
Copy Markdown
Member Author

Thanks for the feedback, everyone. I've disabled the tests I found failing, and created JBS issues where necessary.

Copy link
Copy Markdown
Contributor

@coleenp coleenp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it's only two bugs so not a big effort. Looks good.

@openjdk openjdk bot added the ready Pull request is ready to be integrated label Mar 24, 2026
@Arraying
Copy link
Copy Markdown
Member Author

@plummercj @alexmenkov could I get a quick svc review/look before I integrate? TIA.

Comment on lines +49 to +50
serviceability/sa/TestJmapCore.java 8380779 generic-all
serviceability/sa/TestJmapCoreMetaspace.java 8380779 generic-all
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following tests should use 8379925:

serviceability/sa/TestJmapCore.java
serviceability/sa/TestJmapCoreMetaspace.java
serviceability/sa/ClhsdbDumpheap.java

The other 4 are all separate issues and they don't require -XX:-UseCompressedOops to reproduce. I was actually getting ready to do a problem list update myself (already have the diffs in place) but was just holding off until I could get bugs filed. If you want to file them all under 8379925 for now that is ok. I might change them once I file new CRs, but you should update the CR to not require -XX:-UseCompressedOops.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually the 8380769 CR is correct for TestHeapDumpForLargeArray.java. I misread it as 8380779

Copy link
Copy Markdown

@alexmenkov alexmenkov Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @alexmenkov, thanks for making a comment in an OpenJDK project!

All comments and discussions in the OpenJDK Community must be made available under the OpenJDK Terms of Use. If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please Use "Add GitHub user alexmenkov" for the summary.

If you are not an OpenJDK Author, Committer or Reviewer, simply check the box below to accept the OpenJDK Terms of Use for your comments.

Your comment will be automatically restored once you have accepted the OpenJDK Terms of Use.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either 8379925 or 8379920 is fine. They are dups, so whichever one is kept open.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I filed JDK-8380851 for serviceability/sa/ClhsdbJstackWithConcurrentLock.java

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I filed JDK-8380852 for serviceability/sa/CDSJMapClstats.java

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I filed JDK-8380853 for the serviceability/sa/ClhsdbJhisto.java failure

@plummercj
Copy link
Copy Markdown
Contributor

@Arraying I have a fix for the following for tests, so please don't problem list them. See #2271

resourcehogs/serviceability/sa/TestHeapDumpForLargeArray.java
serviceability/sa/CDSJMapClstats.java
serviceability/sa/ClhsdbJhisto.java
serviceability/sa/ClhsdbJstackWithConcurrentLock.java

@Arraying
Copy link
Copy Markdown
Member Author

@plummercj @alexmenkov thanks for the feedback. I've updated the tests to use 8379925 and removed the ones being addressed in the PR. I'll address the comments in the JBS issues momentarily.

@Arraying Arraying closed this Mar 27, 2026
@Arraying
Copy link
Copy Markdown
Member Author

Arraying commented Mar 27, 2026

I did not mean to close the PR, but if you think it's easier to do the problemlist changes in your respective feature PRs I'm okay to abandon this.

@Arraying Arraying reopened this Mar 27, 2026
Copy link
Copy Markdown
Contributor

@plummercj plummercj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@Arraying
Copy link
Copy Markdown
Member Author

Thanks everyone for the input.
/integrate

@openjdk
Copy link
Copy Markdown

openjdk bot commented Mar 30, 2026

Going to push as commit d3b6dca.
Since your change was applied there have been 239 commits pushed to the lworld branch:

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot added the integrated Pull request has been integrated label Mar 30, 2026
@openjdk openjdk bot closed this Mar 30, 2026
@openjdk openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Mar 30, 2026
@openjdk
Copy link
Copy Markdown

openjdk bot commented Mar 30, 2026

@Arraying Pushed as commit d3b6dca.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integrated Pull request has been integrated

Development

Successfully merging this pull request may close these issues.

4 participants